Skip to content

Fix #48725: Support for flushing in zlib stream #6019

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from

Conversation

cmb69
Copy link
Member

@cmb69 cmb69 commented Aug 19, 2020

When php_zlib_deflate_filter() is called with PSFS_FLAG_FLUSH_INC
but without new buckets being available (e.g. because a user calls
rewind() after writing to the stream), we have to make sure that any
pending data are flushed. This could basically be done like in the
attached patch[1], but that could cause unnessary flushes, which can be
harmful for compression, and adds unnecessary flush markers to the
stream. Thus, we use the php_zlib_filter_data.finished field, which
has not been used for zlib.deflate filters, and properly keep track
of the need to flush.

[1] https://bugs.php.net/patch-display.php?bug_id=48725&patch=zlib-filter-flush-fix.patch&revision=latest

@cmb69 cmb69 added the Bug label Aug 21, 2020
@cmb69
Copy link
Member Author

cmb69 commented Aug 24, 2020

Not sure whom to ping for review. Maybe @sgolemon?

When `php_zlib_deflate_filter()` is called with `PSFS_FLAG_FLUSH_INC`
but without new buckets being available (e.g. because a user calls
`rewind()` after writing to the stream), we have to make sure that any
pending data are flushed.  This could basically be done like in the
attached patch[1], but that could cause unnessary flushes, which can be
harmful for compression, and adds unnecessary flush markers to the
stream.  Thus, we use the `php_zlib_filter_data.finished` field, which
has not been used for `zlib.deflate` filters, and properly keep track
of the need to flush.

[1] <https://bugs.php.net/patch-display.php?bug_id=48725&patch=zlib-filter-flush-fix.patch&revision=latest>
@cmb69 cmb69 changed the base branch from PHP-7.3 to PHP-7.4 November 10, 2020 13:33
@cmb69
Copy link
Member Author

cmb69 commented Nov 10, 2020

I have rebased this onto PHP-7.4, because it's too late for 7.3 fixes.

Still I wonder whether anybody wants to review this.

@php-pulls php-pulls closed this in 20e7532 Dec 8, 2020
@cmb69 cmb69 deleted the cmb/48725 branch December 8, 2020 11:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant